Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vault/allow multiple currency #76

Merged
merged 54 commits into from
Nov 11, 2024
Merged

Vault/allow multiple currency #76

merged 54 commits into from
Nov 11, 2024

Conversation

vuong177
Copy link
Collaborator

@vuong177 vuong177 commented Nov 4, 2024

Close #75

Copy link
Collaborator

@hieuvubk hieuvubk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some comments.
There something about VaultManager.MintAvailable we should go deeper since we allow multi debt denoms. Currently it math.Int so we need to use a unified currency unit for calculations, or have max debt for each debt denom

string denom = 1;
string debt_denom = 1;

string mint_denom = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should rename. Lets say debt and mint denom is same nomUSD

@@ -142,7 +144,9 @@ message VaultLiquidationStatus {
}

message Liquidation {
string denom = 1;
string debt_denom = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be collateral_denom I think

@@ -31,7 +34,8 @@ func (k Keeper) GetNewAuction(ctx context.Context,
if newAuction != nil {
return newAuction, false, nil
}
newAuction, err = k.NewAuction(ctx, startTime, initialPrice, item, targetGoal, vaultId)
newAuction, err = k.NewAuction(ctx, startTime, initialPrice, item, targetGoal, vaultId, vault.Debt.Denom)

if err != nil {
return newAuction, true, err
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if err != nil so the auction should not be created. Let check the bool return

@@ -58,7 +63,7 @@ func (k Keeper) NewAuction(ctx context.Context,
LastDiscountTime: startTime,
Status: types.AuctionStatus_AUCTION_STATUS_ACTIVE,
TargetGoal: targetGoal,
TokenRaised: sdk.NewCoin(vaultstypes.DefaultMintDenom, math.ZeroInt()),
TokenRaised: sdk.NewCoin(mintDenom, math.ZeroInt()),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

if price == nil || price.IsNil() {
return errors.Wrapf(oracletypes.ErrInvalidOracle, "CreateNewVault: can not get price with base %s quote %s", denom, types.DefaultMintDenom)
return errors.Wrapf(oracletypes.ErrInvalidOracle, "CreateNewVault: can not get price with base %s quote %s", denom, types.DefaultMintDenoms)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return errors.Wrapf(oracletypes.ErrInvalidOracle, "CreateNewVault: can not get price with base %s quote %s", denom, types.DefaultMintDenoms)
return errors.Wrapf(oracletypes.ErrInvalidOracle, "CreateNewVault: can not get price with base %s quote %s", denom, mint.Denom)

if price == nil || price.IsNil() {
return errors.Wrapf(oracletypes.ErrInvalidOracle, "MintCoin: can not get price with base %s quote %s", lockedCoin.Denom, types.DefaultMintDenom)
return errors.Wrapf(oracletypes.ErrInvalidOracle, "MintCoin: can not get price with base %s quote %s", lockedCoin.Denom, types.DefaultMintDenoms)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito

// defensive programming: should never happen since when withdraw should always have a valid oracle price
if price == nil || price.IsNil() {
return errors.Wrapf(oracletypes.ErrInvalidOracle, "WithdrawFromVault: can not get price with base %s quote %s", collateral.Denom, types.DefaultMintDenom)
return errors.Wrapf(oracletypes.ErrInvalidOracle, "WithdrawFromVault: can not get price with base %s quote %s", collateral.Denom, types.DefaultMintDenoms)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito

if price == nil || price.IsNil() {
return true, errors.Wrapf(oracletypes.ErrInvalidOracle, "GetLiquidations: can not get price with base %s quote %s", vm.Denom, types.DefaultMintDenom)
return true, errors.Wrapf(oracletypes.ErrInvalidOracle, "GetLiquidations: can not get price with base %s quote %s", vm.Denom, types.DefaultMintDenoms)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito

@vuong177 vuong177 merged commit 2a3b39c into main Nov 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow multiple mint denom
4 participants